Skip to content

ref(settings): convert OwnerInput from class to function component#110200

Merged
JoshuaKGoldberg merged 1 commit intomasterfrom
function-component-owner-input
Mar 10, 2026
Merged

ref(settings): convert OwnerInput from class to function component#110200
JoshuaKGoldberg merged 1 commit intomasterfrom
function-component-owner-input

Conversation

@JoshuaKGoldberg
Copy link
Member

This component had two areas of unused code:

  • Class methods: mentionableUsers, mentionableTeams, and handleAddRule were never used
  • Props: paths and urls were never used in its code

Tested by going to /settings/projects/*/ownership/ and editing the ownership rules.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 9, 2026
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review March 9, 2026 14:42
@JoshuaKGoldberg JoshuaKGoldberg changed the title ref(views): convert OwnerInput from class to function component ref(settings): convert OwnerInput from class to function component Mar 9, 2026
@JoshuaKGoldberg JoshuaKGoldberg requested a review from a team March 9, 2026 15:05
Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 gonna approve this upfront, left some mutation related comments that qualify as additional cleanup rather than the actual conversion

Comment on lines 66 to 67
const api = new Client();
const request = api.requestPromise(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we convert this to using useMutation with fetchMutation as a fn. Example:

mutationFn: (data: AutofixAutomationUpdate) => {
return fetchMutation({
method: 'POST',
url: getApiUrl(
`/organizations/$organizationIdOrSlug/autofix/automation-settings/`,
{
path: {organizationIdOrSlug: organization.slug},
}
),
data,
});
},

},
() => onSave?.(ownership)
);
setHasChanges(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we use a mutation, I feel like this becomes derived state by comparing the mutation data with the stored text:

const hasChanges = mutation.data !== text

@JoshuaKGoldberg
Copy link
Member Author

💯 for sure! I've been hoping for an excuse to ramp up on mutations. Filed ENG-7011 Cleanup followups: OwnerInput switch from api.requestPromise to useMutation and assigned it to myself.

@JoshuaKGoldberg JoshuaKGoldberg merged commit 2d653b5 into master Mar 10, 2026
63 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the function-component-owner-input branch March 10, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants